Fix dockerfile.security.missing-user rules #3448

saghaulor · 2024-08-09T22:32:52Z

A USER directive can appear after the CMD or ENTRYPOINT directive and still be valid
Updated sample dockerfiles with code comments
Relates to Fixed the dockerfile.security.missing-user rule #3438

- A `USER` directive can appear after the `CMD` or` ENTRYPOINT` directive and still be valid - Updated sample dockerfiles with code comments

0xDC0DE · 2024-08-13T09:16:36Z

dockerfile/security/missing-user-entrypoint.dockerfile

+# TODO: metavar ellipses bug, this should be a finding but is a false negative
+# ruleid: missing-user-entrypoint


For failing testcases that are related to the engine instead of the rule, we also have todook and todoruleid. This allows our tests to pass for now, and the rule in its current state to be published. But also allows our developers to verify what the intended behaviour is. If they make an engine update that fixes this problem, they will update the test syntax.

Suggested change

# TODO: metavar ellipses bug, this should be a finding but is a false negative

# ruleid: missing-user-entrypoint

# TODO: metavar ellipses bug, this should be a finding but is a false negative

# todoruleid: missing-user-entrypoint

0xDC0DE · 2024-08-13T09:16:54Z

dockerfile/security/missing-user-entrypoint.fixed.dockerfile

 # ok: missing-user-entrypoint
 ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]


Suggested change

# ok: missing-user-entrypoint

ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]

# todoruleid: missing-user-entrypoint

ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]

0xDC0DE · 2024-08-13T09:17:34Z

dockerfile/security/missing-user-entrypoint.yaml

+        ...
        USER $USER
        ...


Not sure if this can be helped, but starting a pattern with an ellipses is very slow! (I think we have CI checks to not allow this)

To avoid a starting ellipses, maybe we could do something like this?

- pattern: | ENTRYPOINT $...VARS - pattern-not-inside: USER $USER ... ENTRYPOINT $...VARS - pattern-not-inside: CMD $...VARS ... ENTRYPOINT $USER

0xDC0DE · 2024-08-13T09:17:50Z

dockerfile/security/missing-user.dockerfile

+# TODO: metavar ellipses bug, this should be a failure but is a false negative
+# ruleid: missing-user


Suggested change

# TODO: metavar ellipses bug, this should be a failure but is a false negative

# ruleid: missing-user

# TODO: metavar ellipses bug, this should be a failure but is a false negative

# todoruleid: missing-user

0xDC0DE · 2024-08-13T09:22:36Z

dockerfile/security/missing-user.yaml

+        ...
        USER $USER
        ...


To avoid a starting ellipses, maybe we could do something like this?

- pattern: | CMD $...VARS - pattern-not-inside: USER $USER ... CMD $...VARS - pattern-not-inside: CMD $...VARS ... USER $USER

0xDC0DE · 2024-08-13T09:23:54Z

Thanks for your contributions, @saghaulor ! These are meaningful updates, let's try to get that CI to pass so we can merge this PR!

saghaulor · 2024-08-22T17:12:22Z

@0xDC0DE thank you, I have returned from a long needed vacation, I'll try to make these changes soon.

saghaulor · 2024-09-13T20:42:29Z

@0xDC0DE I finally had a chance to look into this more. It seems that this metavariable ellipsis bug is what is preventing us from writing an optimal rule. I tried to write a rule like you specified and it didn't work. The missing-user.fixed.dockerfile ends up in the results, which should not happen.

❯ head -n20 missing-user.yaml
rules:
- id: missing-user
  patterns:
    - pattern: |
        CMD $...VARS
    - pattern-not-inside: |
        USER $USER
        ...
        CMD $...VARS
    - pattern-not-inside: |
        CMD $...VARS
        ...
        USER $USER
  fix: |
    USER non-root
    CMD $...VARS
  message: By not specifying a USER, a program in the container may run as 'root'. This is a security
    hazard. If an attacker can control a process running as root, they may have control over the container.
    Ensure that the last USER in a Dockerfile is a USER other than 'root'.
  severity: ERROR

❯ semgrep scan --config=missing-user.yaml


┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 14 files (only git-tracked) with 1 Code rule:

  CODE RULES
  Scanning 8 files.

  SUPPLY CHAIN RULES

  No rules to run.


  PROGRESS

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00


┌─────────────────┐
│ 2 Code Findings │
└─────────────────┘

    missing-user.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep --config localfile targets
           10┆ CMD semgrep --config localfile targets

    missing-user.fixed.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep
           11┆ CMD semgrep

If we change the rule to remove the metavariable ellipsis behavior, it works as expected, namely, the missing-user.fixed.dockerfile is no longer reporting a finding. Please note that I only changed the second pattern-not-inside clause to remove the metavariable ellipsis behavior as that is what would trigger in the fixed missing-user.fixed.dockerfile.

❯ head -n20 missing-user.yaml
rules:
- id: missing-user
  patterns:
    - pattern: |
        CMD $...VARS
    - pattern-not-inside: |
        USER $USER
        ...
        CMD $...VARS
    - pattern-not-inside: |
        CMD ...
        ...
        USER $USER
  fix: |
    USER non-root
    CMD $...VARS
  message: By not specifying a USER, a program in the container may run as 'root'. This is a security
    hazard. If an attacker can control a process running as root, they may have control over the container.
    Ensure that the last USER in a Dockerfile is a USER other than 'root'.
  severity: ERROR

❯ semgrep scan --config=missing-user.yaml


┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 14 files (only git-tracked) with 1 Code rule:

  CODE RULES
  Scanning 8 files.

  SUPPLY CHAIN RULES

  No rules to run.


  PROGRESS

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00


┌────────────────┐
│ 1 Code Finding │
└────────────────┘

    missing-user.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep --config localfile targets
           10┆ CMD semgrep --config localfile targets

This metavariable ellipsis bug also prevents the Semgrep engine from identifying incorrect patterns within the IDE.

The rule will trigger and highlight code when the Dockerfile specifies something like:

CMD semgrep

But not when it's something like:

CMD semgrep --severity=error

Therefore, I think we need to address the metavariable ellipsis bug before we can fix this rule for good. I believe that there is already an issue open here semgrep/semgrep#9109 regarding this bug. I believe this issue is also related semgrep/semgrep#9726

Please advise.

0xDC0DE · 2024-09-19T06:48:33Z

Thanks for digging into this, @saghaulor . I've pinged some of our engineers to see if they can bump the issues you've linked in priority, but it looks like they've been open for a while.

Fix dockerfile.security.missing-user rules

d486056

- A `USER` directive can appear after the `CMD` or` ENTRYPOINT` directive and still be valid - Updated sample dockerfiles with code comments

0xDC0DE requested changes Aug 13, 2024

View reviewed changes

Merge branch 'develop' into fix_dockerfile.security.missing-user

e5e8d05

Merge branch 'develop' into fix_dockerfile.security.missing-user

c787aa6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dockerfile.security.missing-user rules #3448

Fix dockerfile.security.missing-user rules #3448

saghaulor commented Aug 9, 2024

0xDC0DE Aug 13, 2024

0xDC0DE Aug 13, 2024

0xDC0DE Aug 13, 2024

0xDC0DE Aug 13, 2024

0xDC0DE Aug 13, 2024

0xDC0DE Aug 13, 2024

0xDC0DE commented Aug 13, 2024

saghaulor commented Aug 22, 2024

saghaulor commented Sep 13, 2024

0xDC0DE commented Sep 19, 2024

		# TODO: metavar ellipses bug, this should be a finding but is a false negative
		# ruleid: missing-user-entrypoint

		# ok: missing-user-entrypoint
		ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]

		# TODO: metavar ellipses bug, this should be a failure but is a false negative
		# ruleid: missing-user

Fix dockerfile.security.missing-user rules #3448

Are you sure you want to change the base?

Fix dockerfile.security.missing-user rules #3448

Conversation

saghaulor commented Aug 9, 2024

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE Aug 13, 2024

Choose a reason for hiding this comment

0xDC0DE commented Aug 13, 2024

saghaulor commented Aug 22, 2024

saghaulor commented Sep 13, 2024

0xDC0DE commented Sep 19, 2024